Repository navigation
Fix mixed line endings lost in vendored requirements (#1567) - #1612
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A vendored requirements.txt rewrite split the file with str::lines() and re-joined every line with one terminator. In a file that mixes CRLF and LF lines, every minority-style line was re-terminated on vendor and again on revert, so `vendor --revert` and `remove` reported success but left a file that differs from the original. Vendor, the superseding re-vendor and revert now edit physical lines that keep their own terminator, as the hosted rewriter already does. A vendor line ends the way the pin it replaces did, and every other line is written back byte for byte. Fixes #1567 Assisted-by: Claude Code:claude-opus-5-5
Review follow-up. A hash-pinned requirement that spans several lines with different endings now records each earlier line's ending when it differs from the last line's, so the revert restores the span byte for byte. Ledgers written before this change still revert as before. The ending-preserving line splitter now lives in utils::line_endings, shared with the vendored Cargo.toml writer instead of a second copy. Refs #1567 Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
Labeled Ready for review at head
Generated by Claude Code |
uv 0.13.2 refuses the `environments = []` pylock that Pipenv 2026.8.0 writes, and the unpinned setup-uv steps picked it up. That turned the pipenv:: e2e leg red on main's nightly and in the merge queue. Same change as #1639; it no-ops once that lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EjY8AbwdkRcREjHqS6jcFx
|
The merge queue dropped this PR because I ported #1639's fix, which pins Generated by Claude Code |
|
Wenxin Jiang (@Wenxin-Jiang) this changed after your approval (on Besides merges from main, the only new commit is 9f0bad3: the CI-only uv pin from #1639, byte-for-byte the same 12 added lines ( If that's fine with you, re-approving Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9f0bad3. Configure here.
|
Ready for review re-confirmed at the new head (burn-down agent).
Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1567
Summary
A vendored rewrite of a
requirements.txt(or a-rinclude) thatmixes CRLF and LF lines no longer re-terminates lines it never touched.
vendor --revertandremovenow restore the file byte for byte.Root cause
The three vendored requirements writers in
vendor/pypi_requirements.rs(
plan_requirements,plan_rewirefor the superseding re-vendor, andrevert_requirements) split the file withstr::lines(), which dropsevery
\r. They then re-joined it with a single terminator, the file'smajority ending. So every minority-style line changed on vendor and again
on revert, while both reported success.
Fix
(
Line { text, eol }), the way the hosted rewriter(
patch/redirect/requirements.rs) already does. A vendor line endsthe way the pin it replaces ended, and every other line is written back
unchanged.
\-continuation) records eachearlier line's ending when it differs from the span's last line. Then
even a continuation that mixes endings internally reverts exactly. A
uniform span records plain texts as before, so ledgers for ordinary
files don't change, and ledgers written before this PR still revert.
utils::line_endings::split_lines,shared with the vendored
Cargo.tomlwriter (vendor/cargo_manifest.rs),which had its own copy.
Tests (per issue)
vendor --revert/removecan't restore the file #1567:mixed_line_endings_survive_vendor_and_revertcovers the LF-majorityand CRLF-majority repros from the issue, plus a CRLF pin before an
unterminated last line.
mixed_line_endings_around_a_continuation_span,a_continuation_span_with_mixed_endings_reverts_byte_identically,mixed_line_endings_survive_rewire_and_revertandmixed_line_endings_survive_a_transitive_appendcover the otherwriters, and
a_uniform_span_records_plain_textscovers theunchanged ledger.
main's writer, the first three new tests failed withexactly the issue's symptom, for example
left: "idna==3.7\n./…whl # socket-patch vendor: six==1.16.0\nrequests==2.31.0\n# tail\n"against the expected
"idna==3.7\r\n…\nrequests==2.31.0\r\n# tail\n".They pass with this change.
Verification run locally
cargo fmt --all -- --check: passcargo clippy --workspace --all-features -- -D warnings: passcargo test -p socket-patch-core --all-features: 6540 passed. 4 failed,and the same 4 fail on
main: they are read-only-directory tests thatcan't fail a write when the sandbox runs as root (
wire_failure_rolls_back_already_written_files,wire_write_failure_maps_error_and_leaves_lock_untouched,relax_loop_must_not_traverse_symlinked_root,an_unremovable_hidden_lock_keeps_every_store_entry). CI runs as non-root.cargo test -p socket-patch-cli --all-features --lib: 921 passede2e_vendor_pypi_build -- --include-ignored: 55 passed.in_process_vendor_pypi_takeover,mode_migration_pypi,scan_vendor_requirements_unwiredandscan_requirements_lock_onlyall pass.
e2e_vendor_cargo_build -- --include-ignored: 40 passed(shared splitter).
cargo test --workspacelocally: linking everyCLI test binary filled the sandbox disk. CI covers it.
npm/,pypi/,gem/don't touch this path)./code-review high findings
Fixed: the mixed-ending continuation span, the duplicated splitter,
missing tests for the span and append shapes, and simplified restore
logic. Not fixed:
Added) line in a root file that had nofinal newline leaves the separator wire added. This predates this PR
and is unrelated to mixed endings. The existing test
transitive_append_to_root_without_trailing_newlinedocuments it. Afix needs a new record shape for
Added, which other readers(
lock_inventory/recover.rs) key on, so it belongs in its own change.and the extra copy isn't measurable next to the I/O.
🤖 Generated with Claude Code
Note
Medium Risk
Changes vendored requirements file rewrite/revert behavior on disk; incorrect edge-case handling could corrupt user requirements files, though coverage is extensive and backward-compatible ledger revert is preserved.
Overview
Fixes #1567 by changing vendored PyPI
requirements.txtwiring so mixed CRLF/LF files stay byte-identical on lines that are not edited.Previously,
plan_requirements,plan_rewire, andrevert_requirementsusedstr::lines()and re-joined with a single file-wide terminator, which re-terminated minority-style lines on both vendor and revert. They now edit physical lines (Line { text, eol }), join with per-line endings, and record span metadata so hash-pinned\-continuations with mixed internal endings revert exactly; older ledgers without embedded endings still revert using the vendor line’s terminator.utils::line_endings::split_linesis added andcargo_manifest.rsdrops its duplicate local splitter in favor of the shared helper.CI pins
uvto 0.13.1 inci.ymlandpipenv-compatibility.ymlbecause 0.13.2 rejects Pipenv pylocks withenvironments = [].New unit tests cover mixed endings across vendor, revert, rewire, transitive append, and continuation spans.
Reviewed by Cursor Bugbot for commit 9f0bad3. Configure here.
Generated by Claude Code